Skip to content

Feat/multi tenant#73

Merged
qnen merged 6 commits intodevelopfrom
feat/multi-tenant
Feb 27, 2026
Merged

Feat/multi tenant#73
qnen merged 6 commits intodevelopfrom
feat/multi-tenant

Conversation

@qnen
Copy link
Contributor

@qnen qnen commented Feb 27, 2026

Pull Request Checklist

Pull Request Type

  • Feature
  • Fix
  • Refactor
  • Pipeline
  • Tests
  • Documentation
  • Helm

Checklist

Please check each item after it's completed.

  • I have tested these changes locally.
  • I have updated the documentation accordingly.
  • I have added necessary comments to the code, especially in complex areas.
  • I have ensured that my changes adhere to the project's coding standards.
  • I have checked for any potential security issues.
  • I have ensured that all tests pass.
  • I have updated the version appropriately (if applicable).
  • I have confirmed this code is ready for review.

Additional Notes

Obs: Please, always remember to target your PR to develop branch instead of main.

qnen and others added 4 commits February 26, 2026 16:50
- 17 test functions with 40+ subtests covering both middleware files
- gRPC: stripBearer, policyForMethod, grpcErrorFromHTTP, extractTokenFromMD,
  SubFromMetadata, NewGRPCAuthUnaryPolicy integration
- HTTP: checkAuthorization subject construction for normal-user and
  application tokens, mock server integration, error handling
- Documents known bug: application tokens hardcode "admin/" prefix
  instead of using owner claim from JWT
- Add testify v1.11.1 dependency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add extractTenantClaims to parse tenantId/tenantSlug/owner from JWT
- Propagate tenant metadata (md-tenant-id, md-tenant-slug, md-tenant-owner)
  in unary interceptor when MULTI_TENANT_ENABLED=true
- Add NewGRPCAuthStreamPolicy streaming interceptor mirroring unary behavior
- Add wrappedServerStream to override context for streaming calls
- Add tests for extraction, propagation, and streaming interceptor

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace NewTracerFromContext + NewHeaderIDFromContext with the unified NewTrackingFromContext across HTTP and gRPC middleware.

X-Lerian-Ref: 0x1
@qnen qnen self-assigned this Feb 27, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 27, 2026

Walkthrough

Adds multi-tenant support to gRPC auth middleware by extracting tenant claims from JWTs and injecting them into gRPC metadata; adds a streaming auth interceptor NewGRPCAuthStreamPolicy that mirrors unary policy behavior and a wrappedServerStream to preserve propagated context. Refactors tracing/request ID retrieval to use commons.NewTrackingFromContext and adds error-handling fallback to use a NoneLogger when logger initialization fails. Introduces extensive tests for unary and streaming middleware paths and updates Go toolchain and dependencies in go.mod.

Sequence Diagram

sequenceDiagram
    participant Client as gRPC Client
    participant Middleware as gRPC Auth Middleware
    participant AuthClient as Auth Service (HTTP)
    participant JWT as JWT Parser
    participant Metadata as gRPC Metadata
    participant Downstream as Downstream Service

    Client->>Middleware: gRPC request (md-auth token)
    Middleware->>Metadata: extract token
    Middleware->>JWT: parse token (ParseUnverified)
    JWT-->>Middleware: claims
    alt MULTI_TENANT_ENABLED
        Middleware->>JWT: extract tenantId/tenantSlug/owner
        JWT-->>Middleware: tenant claims
        Middleware->>Metadata: inject md-tenant-id/md-tenant-slug/md-tenant-owner
    end
    Middleware->>AuthClient: checkAuthorization(subject, policy) (HTTP)
    AuthClient-->>Middleware: AuthResponse (authorized / denied / error)
    alt authorized
        Middleware->>Downstream: forward request with metadata (propagated context/request-id)
        Downstream-->>Client: response
    else denied or error
        Middleware-->>Client: gRPC error (unauthenticated/permission denied/internal)
    end
Loading
🚥 Pre-merge checks | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description follows the template structure but is largely incomplete, with no specific implementation details, only the checklist template completed and no items marked as done. Complete the PR description by adding specific details about what multi-tenant support was implemented, how it works, testing status, and which checklist items were actually completed.
Title check ❓ Inconclusive The title 'Feat/multi tenant' is vague and generic, using only a prefix and feature area without describing the actual implementation or core change. Revise the title to clearly describe the multi-tenant feature implementation, e.g., 'Add multi-tenant support to gRPC middleware' or similar.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@auth/middleware/middleware_test.go`:
- Line 10: The test imports the wrong major version of commons log causing type
mismatch with AuthClient.Logger; change the import from
"github.com/LerianStudio/lib-commons/v2/commons/log" to the v3 module (matching
production), and update any testLogger methods (the ones returning log.Logger)
to use the v3 log.Logger type so their return types match AuthClient.Logger.

In `@auth/middleware/middlewareGRPC_test.go`:
- Around line 60-66: The test and middleware currently treat an "Authorization:
Bearer " header as the literal "Bearer" token, which hides a parsing bug; update
the token-parsing logic (stripBearer) so that when the header is exactly the
bearer prefix with no token (e.g., "Bearer " or "bearer ") it returns an empty
string, and update the GRPC auth middleware handler (the code path that inspects
the token and maps to gRPC status) to treat an empty token as unauthenticated
and return codes.Unauthenticated rather than proceeding; also change the failing
tests in middlewareGRPC_test.go (including the case named
"bearer_prefix_with_no_token_returns_bearer_literal" and similar cases around
lines referenced) to expect an empty credential and an Unauthenticated outcome.

In `@auth/middleware/middlewareGRPC.go`:
- Around line 102-121: Extract the duplicated MULTI_TENANT_ENABLED + metadata
mutation block into a single helper function (e.g., propagateTenantClaims(ctx
context.Context, token *jwt.Token) context.Context) in middlewareGRPC.go; move
the os.Getenv("MULTI_TENANT_ENABLED") check, tenant extraction
(extractTenantClaims), metadata.Copy/Set calls for "md-tenant-id",
"md-tenant-slug", "md-tenant-owner", and metadata.NewIncomingContext into that
helper, and then replace the duplicated blocks in both the unary interceptor and
streaming interceptor with a call to propagateTenantClaims(ctx, token) (or
similar) to return the updated context. Ensure the helper handles empty claim
values and returns the original context when multi-tenant is disabled or no
tenant claims are present.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a19c60 and 37145a5.

📒 Files selected for processing (5)
  • auth/middleware/middleware.go
  • auth/middleware/middlewareGRPC.go
  • auth/middleware/middlewareGRPC_test.go
  • auth/middleware/middleware_test.go
  • go.mod

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@auth/middleware/middleware_test.go`:
- Around line 436-449: Update the TestAuthResponse_JSONRoundTrip to include the
Timestamp field: import "time", set original.Timestamp (e.g., time.Now().UTC()),
marshal/unmarshal as before, and assert that decoded.Timestamp equals
original.Timestamp using time.Time's Equal method (or compare UnixNano) to
ensure the full struct round-trip is covered; reference the
TestAuthResponse_JSONRoundTrip function and the AuthResponse.Timestamp field
when making the change.

In `@auth/middleware/middleware.go`:
- Around line 59-64: The wrapped error is created but never emitted; update the
error branch after calling zap.InitializeLoggerWithError() so the failure is
logged before falling back to &log.NoneLogger{} (e.g., wrap the error with
fmt.Errorf("failed to initialize logger: %w", err) and write it to stderr with
fmt.Fprintln(os.Stderr, wrappedErr) or similar), and add the "os" import; ensure
you still assign l = &log.NoneLogger{} afterwards so InitializeLoggerWithError,
l, and log.NoneLogger are used correctly.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 37145a5 and 4b6b5c5.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • auth/middleware/middleware.go
  • auth/middleware/middleware_test.go
  • go.mod

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
auth/middleware/middleware.go (1)

60-65: ⚠️ Potential issue | 🟡 Minor

Logging to NoneLogger is ineffective—error context is still lost.

The error is being logged via l.Errorf(), but l is assigned &log.NoneLogger{} before that call. By definition, NoneLogger discards all log output, so the failure message is never emitted. This doesn't resolve the original concern about losing error context.

Log to os.Stderr before assigning the fallback logger, or use a simple fmt.Fprintf since no working logger is available at that point.

🔧 Proposed fix
 	} else {
 		l, err = zap.InitializeLoggerWithError()
 		if err != nil {
+			fmt.Fprintf(os.Stderr, "failed to initialize logger, falling back to NoneLogger: %v\n", err)
 			l = &log.NoneLogger{}
-
-			l.Errorf("failed to initialize logger, using NoneLogger: %v\n", err)
 		}
 	}

Add "os" to the imports:

 import (
 	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"net/http"
+	"os"
 	"time"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@auth/middleware/middleware.go` around lines 60 - 65, The current error
handling assigns l = &log.NoneLogger{} before calling l.Errorf, so the
initialization error is never emitted; change the sequence so you write the
error directly to stderr (e.g., use fmt.Fprintf(os.Stderr, ...) or log.Print to
os.Stderr) immediately after zap.InitializeLoggerWithError() returns an error,
then assign the fallback l = &log.NoneLogger{} afterward; update the imports to
include "os" (and "fmt" if using fmt.Fprintf) and replace the l.Errorf(...) call
with a direct stderr write referencing the error from
InitializeLoggerWithError() so the failure context is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@auth/middleware/middleware.go`:
- Around line 60-65: The current error handling assigns l = &log.NoneLogger{}
before calling l.Errorf, so the initialization error is never emitted; change
the sequence so you write the error directly to stderr (e.g., use
fmt.Fprintf(os.Stderr, ...) or log.Print to os.Stderr) immediately after
zap.InitializeLoggerWithError() returns an error, then assign the fallback l =
&log.NoneLogger{} afterward; update the imports to include "os" (and "fmt" if
using fmt.Fprintf) and replace the l.Errorf(...) call with a direct stderr write
referencing the error from InitializeLoggerWithError() so the failure context is
preserved.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6b5c5 and 78150e9.

📒 Files selected for processing (1)
  • auth/middleware/middleware.go

@qnen qnen merged commit 44e4e6e into develop Feb 27, 2026
3 checks passed
@qnen qnen deleted the feat/multi-tenant branch February 27, 2026 17:58
@lerian-studio-midaz-push-bot
Copy link

🎉 This PR is included in version 2.5.0-beta.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants